Skip to content

ASN.1 print utility: asn1#6352

Merged
dgarske merged 1 commit intowolfSSL:masterfrom
SparkiDev:asn1_print
May 4, 2023
Merged

ASN.1 print utility: asn1#6352
dgarske merged 1 commit intowolfSSL:masterfrom
SparkiDev:asn1_print

Conversation

@SparkiDev
Copy link
Copy Markdown
Contributor

Description

Add a utility to parse ASN.1 files.

Testing

How did you test?

Checklist

  • added tests
  • updated/added doxygen
  • updated appropriate READMEs
  • Updated manual and documentation

Copy link
Copy Markdown
Contributor

@JacobBarthelmeh JacobBarthelmeh left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice addition! Not sure how thorough the review should be here with a utility app and initial addition? If having complete review, than as it is it does not stand up to fuzz testing (it hits some crashes).

./configure --enable-all --enable-static --disable-shared CC="afl-gcc -g"
sed -i 's/-Werror//' Makefile
make
mkdir output
afl-fuzz -i ./certs -o ./output ./util/asn1 @@

That's before moving on to libfuzzer/address sanitize smoke tests.

Comment thread util/asn1.c Outdated
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

util/asn1.c:714:24: error: implicit conversion loses integer precision: 'size_t'
      (aka 'unsigned long') to 'word32' (aka 'unsigned int')
      [-Werror,-Wshorten-64-to-32]
    while ((read_len = fread(data + len, 1, DATA_INC_LEN, fp)) != 0) {
                     ~ ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
1 error generated.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

fixed

@dgarske dgarske removed their assignment Apr 28, 2023
Copy link
Copy Markdown
Member

@dgarske dgarske left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Great utility! Any reason these isn't in the wolfCLU as an asn1parse feature instead?
If it stays here please add some usage examples and README.md. Example: ./util/asn1 ./certs/server-cert.der or ./util/asn1 -p ./certs/server-cert.pem.

Comment thread util/asn1.c Outdated
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

malloc failure check?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

fixed

Comment thread util/asn1.c Outdated
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Help option(s)? -? --help ?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

fixed

Comment thread util/asn1.c Outdated
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Same malloc failure check?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

fixed

@anhu
Copy link
Copy Markdown
Member

anhu commented Apr 28, 2023

Oh man!! This is going to be a game changer!! Wish I had this about 2 weeks ago.

@JacobBarthelmeh JacobBarthelmeh removed their assignment May 1, 2023
Comment thread util/asn1.c Outdated
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You might consider exposing this through some optional API in the library that can optionally be enabled at build-time. Then it would be easier for wolfCLU to consume. This util application can still exist. Although instead of adding another new directory consider putting in examples or scripts.

Copy link
Copy Markdown
Contributor Author

@SparkiDev SparkiDev May 2, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'll put PrintAllAsn1 and below into asn.c.
Putting into examples/asn1.

@SparkiDev SparkiDev marked this pull request as draft May 2, 2023 21:09
@SparkiDev SparkiDev force-pushed the asn1_print branch 4 times, most recently from ed0dbc4 to 5459b87 Compare May 3, 2023 09:12
@SparkiDev SparkiDev assigned dgarske and JacobBarthelmeh and unassigned SparkiDev May 3, 2023
Copy link
Copy Markdown
Member

@dgarske dgarske left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Very good updates! Getting close!

Comment thread examples/asn1/asn1.c Outdated
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

examples/asn1/asn1.c: In function ‘int ReadFile(FILE*, unsigned char**, word32*)’:
examples/asn1/asn1.c:68:33: error: invalid conversion from ‘void*’ to ‘unsigned char*’ [-fpermissive]
   68 |     unsigned char* data = malloc(DATA_INC_LEN);
      |                           ~~~~~~^~~~~~~~~~~~~~
      |                                 |
      |                                 void*
examples/asn1/asn1.c:84:24: error: invalid conversion from ‘void*’ to ‘unsigned char*’ [-fpermissive]
   84 |             p = realloc(data, len + DATA_INC_LEN);
      |                 ~~~~~~~^~~~~~~~~~~~~~~~~~~~~~~~~~
      |                        |
      |                        void*
make[2]: *** [Makefile:5884: examples/asn1/as

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed.

Comment thread configure.ac Outdated
Comment thread wolfcrypt/src/asn.c Outdated
Comment thread wolfcrypt/src/asn.c Outdated
Comment thread wolfcrypt/src/asn.c Outdated
@dgarske dgarske assigned SparkiDev and unassigned JacobBarthelmeh and dgarske May 3, 2023
@dgarske dgarske marked this pull request as ready for review May 3, 2023 23:12
@dgarske
Copy link
Copy Markdown
Member

dgarske commented May 3, 2023

New one: wolfcrypt/src/asn.c:296:20: error: ‘TagString’ defined but not used [-Werror=unused-function] [992](https://github.com/wolfSSL/wolfssl/actions/runs/4877314346/jobs/8701836465?pr=6352#step:2:1006) 296 | static const char* TagString(byte tag) [993](https://github.com/wolfSSL/wolfssl/actions/runs/4877314346/jobs/8701836465?pr=6352#step:2:1007) | ^~~~~~~~~

Comment thread configure.ac Outdated
Copy link
Copy Markdown
Member

@dgarske dgarske left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks like one place missed for WOLFSSL_NO_ASN_PRINT. Otherwise golden!

Comment thread wolfssl/wolfcrypt/asn.h Outdated
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks like there are still some WOLFSSL_NO_ASN_PRINT?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

fixed

New API to parse and print DER/BER data from a buffer.
Add an example to parse DER, Base64 and PEM files and print out ASN.1 items.
@dgarske dgarske self-assigned this May 4, 2023
@SparkiDev SparkiDev removed their assignment May 4, 2023
@dgarske
Copy link
Copy Markdown
Member

dgarske commented May 4, 2023

Retest this please

1 similar comment
@dgarske
Copy link
Copy Markdown
Member

dgarske commented May 4, 2023

Retest this please

@dgarske dgarske merged commit 6e572cc into wolfSSL:master May 4, 2023
@gvanem
Copy link
Copy Markdown

gvanem commented Aug 4, 2023

This program does not compile on Windows. From clang-cl:

examples/asn1/asn1.c(64,12): error: conflicting types for 'ReadFile'
static int ReadFile(FILE* fp, unsigned char** pdata, word32* plen)
           ^
f:\gv\WinKit\Include\10.0.22621.0\um\fileapi.h(979,1): note: previous declaration is here
ReadFile(
^
examples/asn1/asn1.c(125,33): error: too few arguments to function call, expected 5, have 3
    if (ReadFile(fp, &data, &len) != 0) {
        ~~~~~~~~                ^
f:\gv\WinKit\Include\10.0.22621.0\um\fileapi.h(979,1): note: 'ReadFile' declared here
ReadFile(
^
...

An easy fix for me was:

--- a/examples/asn1/asn1.c 2023-08-04 09:51:41
+++ b/examples/asn1/asn1.c 2023-08-04 13:48:06
@@ -50,6 +50,9 @@
 /* ASN.1 parsing state. */
 static Asn1 asn1;

+#undef  ReadFile
+#define ReadFile(fp, data, len)  asn1_ReadFile (fp, data, len)
+
 /* Read the contents of a file into a dynamically allocated buffer.
  *
  * Uses realloc as input may be stdin.

So now a asn1.exe -p ./certs/server-cert.pem seems to work:

   0: 4 [1256]  (0) +SEQUENCE
   4: 4 [ 976]  (1) +SEQUENCE
   8: 2 [   3]  (2) +[0]
  10: 2 +   1   (3)  INTEGER         :02
  13: 2 +   1   (2)  INTEGER         :01
  16: 2 [  13]  (2) +SEQUENCE
  18: 2 +   9   (3)  OBJECT ID       :(0x28f) sha256WithRSAEncryption
  29: 2 +   0   (3)  NULL
  31: 3 [ 148]  (2) +SEQUENCE
  34: 2 [  11]  (3) +SET
  36: 2 [   9]  (4) +SEQUENCE
  38: 2 +   3   (5)  OBJECT ID       :(0x5f) countryName
  43: 2 +   2   (5)  PrintableString :US
  47: 2 [  16]  (3) +SET
  49: 2 [  14]  (4) +SEQUENCE
  51: 2 +   3   (5)  OBJECT ID       :(0x61) stateOrProvinceName
  56: 2 +   7   (5)  UT8String       :Montana

@anhu
Copy link
Copy Markdown
Member

anhu commented Aug 4, 2023

Hi @gvanem ,
Thank you for letting us know. I propose changing the function name to something more specific. I'll post and update here once I have put up a new PR.

@anhu
Copy link
Copy Markdown
Member

anhu commented Aug 4, 2023

@gvanem , please see #6673 .

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants